Fix Flaky results on Library and Gateway controller test#2039
Conversation
We are getting flaky results on this test and adding Eventually wrapper can help us to avoid this situations Signed-off-by: Francisco Herrera <fjglira@gmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2039 +/- ##
=======================================
Coverage 76.57% 76.57%
=======================================
Files 58 58
Lines 3181 3181
=======================================
Hits 2436 2436
Misses 608 608
Partials 137 137
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| g.Expect(ready).To(BeTrue(), "curl-client pod is not ready") | ||
| }).Should(Succeed()) | ||
| Success("All pods in gateway namespace are ready") | ||
| Success("Curl client pod is ready") |
There was a problem hiding this comment.
Need to check if this work, because now we are only validating that curl pod is running, but the previous step validates that all the gateway deploy are Ready
There was a problem hiding this comment.
Pull request overview
This PR addresses flaky E2E failures in the Sail Operator test suite by making readiness/drift assertions more resilient to eventual consistency and background reconciliation, specifically in the Library reconciliation and Gateway controller E2E tests.
Changes:
- Wrap the ValidatingWebhookConfiguration drift mutation in an
Eventuallyblock to tolerate concurrent reconciler updates. - Narrow the Gateway controller “pod ready” check to the specific
curl-clientpod rather than requiring all pods in the gateway namespace to be ready.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/e2e/library/library_reconcile_test.go | Retries webhook drift mutation to reduce flakes from concurrent reconciliation. |
| tests/e2e/gatewaycontroller/gateway_controller_test.go | Makes the curl-client readiness check more targeted to avoid unrelated pod readiness causing flakes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The background reconciler may update the webhook concurrently; retry on conflict. | ||
| Eventually(func(g Gomega) { | ||
| webhook := &admissionv1.ValidatingWebhookConfiguration{} | ||
| g.Expect(cl.Get(ctx, webhookKey, webhook)).To(Succeed()) | ||
| webhook.Webhooks[0].Name = "xyz.xyz.xyz" | ||
| webhook.Webhooks[0].FailurePolicy = ptr.Of(admissionv1.Fail) | ||
| g.Expect(cl.Update(ctx, webhook)).To(Succeed()) |
|
Looks ok to me. |
Signed-off-by: Francisco Herrera <fjglira@gmail.com>
|
I had to do the change manually |
|
Seems that is an extra flaky test now: Checkingto see if I can fix this in the same PR |
Signed-off-by: Francisco Herrera <fjglira@gmail.com>
|
@MaxBab it failed before as my last messages shown, I pushed a new commit with a new validate connectivity logic to avoid this kind of issues |
* upstream/main: Fix Flaky results on Library and Gateway controller test (istio-ecosystem#2039) docs: fix inaccurate "ambient mode only supports InPlace" update-strategy note (istio-ecosystem#2005) test(e2e): write debug info to artifacts only, print when E2E_PRINT_DEBUG=true (istio-ecosystem#2031) fix(e2e): initialize controller-runtime logger in ambient suite (istio-ecosystem#2030) test: Fix PodSecurity error on gateway test while running on OCP clusters (istio-ecosystem#2025) test: organization of labels used on e2e test (istio-ecosystem#2026) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2024) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2020) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2013) fix(install): align library drift watch with Helm label (istio-ecosystem#2017) Sail library crds fixes (istio-ecosystem#2014) test(revision): fix flaky TestPruneInactive requeueAfter assertion (istio-ecosystem#2015) Add E2E migration test (istio-ecosystem#1974) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2010) Automate FBC upgrade graph flow during release workflow (istio-ecosystem#1994) Improve tests logs gathering (istio-ecosystem#2007) Fix race condition in `ToDiscoveryClient` (istio-ecosystem#2003) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2006) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2000)
* upstream/main: Fix update-deps job (istio-ecosystem#2047) Fix Flaky results on Library and Gateway controller test (istio-ecosystem#2039) docs: fix inaccurate "ambient mode only supports InPlace" update-strategy note (istio-ecosystem#2005) test(e2e): write debug info to artifacts only, print when E2E_PRINT_DEBUG=true (istio-ecosystem#2031) fix(e2e): initialize controller-runtime logger in ambient suite (istio-ecosystem#2030) test: Fix PodSecurity error on gateway test while running on OCP clusters (istio-ecosystem#2025) test: organization of labels used on e2e test (istio-ecosystem#2026) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2024) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2020) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2013) fix(install): align library drift watch with Helm label (istio-ecosystem#2017) Sail library crds fixes (istio-ecosystem#2014) test(revision): fix flaky TestPruneInactive requeueAfter assertion (istio-ecosystem#2015) Add E2E migration test (istio-ecosystem#1974) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2010) Automate FBC upgrade graph flow during release workflow (istio-ecosystem#1994) Improve tests logs gathering (istio-ecosystem#2007) Fix race condition in `ToDiscoveryClient` (istio-ecosystem#2003) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2006) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#2000)
We are getting flaky results on this test and adding Eventually wrapper can help us to avoid this situations
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #2038
Related Issue/PR #
Additional information: